ui/studio_html_test.py (1)

15-25: Add a regression assertion for missing-storage default volume.
Given the first-run guard in the volume persistence logic, add a small test here that asserts the HTML contains the default-volume seeding branch (e.g., null/undefined storage checks + DEFAULT volume assignment). This will protect against first-run mute regressions.

As per coding guidelines: Add or update tests for every behavior change and bug fix; include at least one success-path test, one regression/edge-case test for the bug being fixed, and one non-target behavior check when relevant.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/studio_html_test.py` around lines 15 - 25, Add a new regression test in
ui/studio_html_test.py (e.g., test_volume_defaults_on_missing_storage) that
asserts the rendered HTML contains the first-run/default-volume seeding branch:
look for references such as localStorage.getItem('volume') being null/undefined
checks and assignment/usage of DEFAULT volume (e.g., DEFAULT_VOLUME or a default
numeric value) and the code path that applies that default via
applyPreferredVolumeToAudio/audioEl (similar to existing
test_volumechange_listener_guards_non_trusted_events which checks
audioEl.addEventListener('volumechange'... and isTrustedUserEvent). Ensure the
new test asserts the presence of the null/undefined storage guard plus the
default assignment string so first-run mute regressions are caught.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

acestep/ui/gradio/events/results/generation_info.py (1)

25-25: ⚠️ Potential issue | 🟡 Minor

Return type annotation still missing on clear_audio_outputs_for_new_generation.

This was previously confirmed as addressed but is absent from the current code.

🛠️ Proposed fix

-def clear_audio_outputs_for_new_generation():
+def clear_audio_outputs_for_new_generation() -> tuple[Any, ...]:

Also ensure Any is imported from typing (already present in the file).

As per coding guidelines: "Add type hints for new/modified functions when practical in Python".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@acestep/ui/gradio/events/results/generation_info.py` at line 25, Add a return
type annotation to the function clear_audio_outputs_for_new_generation by
updating its signature to include -> None (i.e., def
clear_audio_outputs_for_new_generation() -> None:) to satisfy the project typing
guidelines; confirm Any is already imported from typing (no additional imports
required) and run type checks to ensure no other signatures were missed.

acestep/ui/gradio/events/results/audio_playback_updates_test.py (1)

8-27: ⚠️ Potential issue | 🟡 Minor

Return type hint on _load_module and _FakeGr.update still missing; no spec-load guard.

Both were confirmed as addressed in a prior review round but are absent from the current code. The type: ignore[union-attr] on line 13 also suppresses what should be a handled error path.

🛠️ Proposed fix

+from types import ModuleType
+from typing import Any
+
-def _load_module():
+def _load_module() -> ModuleType:
     """Load the target module directly by file path for isolated testing."""
     module_path = Path(__file__).with_name("audio_playback_updates.py")
     spec = importlib.util.spec_from_file_location("audio_playback_updates", module_path)
+    if spec is None or spec.loader is None:
+        raise RuntimeError(f"Failed to create module spec for {module_path}")
     module = importlib.util.module_from_spec(spec)
-    spec.loader.exec_module(module)  # type: ignore[union-attr]
+    spec.loader.exec_module(module)
     return module

 class _FakeGr:
     """Minimal Gradio-like stub exposing ``update``."""

     `@staticmethod`
-    def update(**kwargs):
+    def update(**kwargs: Any) -> dict[str, Any]:
         """Return kwargs for direct assertion in tests."""
         return kwargs

As per coding guidelines: "Add type hints for new/modified functions when practical in Python" and "Handle errors explicitly in Python; avoid bare except".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@acestep/ui/gradio/events/results/audio_playback_updates_test.py` around lines
8 - 27, Add explicit return type hints and an explicit loader check: annotate
_load_module() to return types.ModuleType, and annotate _FakeGr.update(...) to
return dict[str, typing.Any]; inside _load_module() check that spec and
spec.loader are not None before calling spec.loader.exec_module and raise a
clear exception if they're missing (remove the type: ignore[union-attr] and
handle the error path explicitly). Use references to _load_module, _MODULE,
build_audio_slot_update and _FakeGr.update to locate where to add the
annotations and the spec-loader guard.

acestep/ui/gradio/events/results/generation_info_test.py (1)

13-31: ⚠️ Potential issue | 🟠 Major

sys.modules stubs still leak globally — patch.dict wrapper not applied.

The function directly mutates sys.modules (lines 17–25) and calls spec.loader.exec_module outside any context manager, so stub entries persist after _load_module() returns and bleed into subsequent test modules. The previously proposed patch.dict fix was confirmed as addressed in an earlier commit but does not appear in the current code.

Additionally _load_module is missing its -> types.ModuleType return type hint, and there is no guard for spec is None / spec.loader is None.

🛠️ Proposed fix

-def _load_module():
+def _load_module() -> types.ModuleType:
     """Load target module directly by file path for isolated testing."""
+    stub_modules: dict[str, types.ModuleType] = {
+        "acestep": types.ModuleType("acestep"),
+        "acestep.ui": types.ModuleType("acestep.ui"),
+        "acestep.ui.gradio": types.ModuleType("acestep.ui.gradio"),
+        "acestep.ui.gradio.i18n": types.ModuleType("acestep.ui.gradio.i18n"),
+    }
+    stub_modules["acestep.ui.gradio.i18n"].t = lambda key, **_kwargs: key
+    stub_modules["acestep"].ui = stub_modules["acestep.ui"]
+    stub_modules["acestep.ui"].gradio = stub_modules["acestep.ui.gradio"]
+    stub_modules["acestep.ui.gradio"].i18n = stub_modules["acestep.ui.gradio.i18n"]
-    acestep_pkg = sys.modules.setdefault("acestep", types.ModuleType("acestep"))
-    ui_pkg = sys.modules.setdefault("acestep.ui", types.ModuleType("acestep.ui"))
-    gradio_pkg = sys.modules.setdefault("acestep.ui.gradio", types.ModuleType("acestep.ui.gradio"))
-    i18n_mod = types.ModuleType("acestep.ui.gradio.i18n")
-    i18n_mod.t = lambda key, **_kwargs: key
-    sys.modules["acestep.ui.gradio.i18n"] = i18n_mod
-    acestep_pkg.ui = ui_pkg
-    ui_pkg.gradio = gradio_pkg
-    gradio_pkg.i18n = i18n_mod

     module_path = Path(__file__).with_name("generation_info.py")
     spec = importlib.util.spec_from_file_location("generation_info", module_path)
+    if spec is None or spec.loader is None:
+        raise RuntimeError(f"Failed to create module spec for {module_path}")
     module = importlib.util.module_from_spec(spec)
-    spec.loader.exec_module(module)  # type: ignore[union-attr]
+    with patch.dict(sys.modules, stub_modules):
+        spec.loader.exec_module(module)
     return module

As per coding guidelines: "Avoid hidden state and unintended side effects in Python code" and "Add type hints for new/modified functions when practical in Python".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@acestep/ui/gradio/events/results/generation_info_test.py` around lines 13 -
31, The helper _load_module currently mutates sys.modules globally and executes
spec.loader.exec_module without guards or cleanup; change it to use
unittest.mock.patch.dict as a context manager to temporarily inject the stub
modules into sys.modules (so entries don't leak), add a return type hint "->
types.ModuleType" on _load_module, and validate the
importlib.util.spec_from_file_location result and spec.loader (raise a clear
RuntimeError if spec or spec.loader is None) before calling
spec.loader.exec_module(module); reference symbols: _load_module, sys.modules,
patch.dict, importlib.util.spec_from_file_location, spec.loader.exec_module, and
types.ModuleType.

acestep/ui/gradio/interfaces/audio_player_preferences.js (1)

76-77: ⚠️ Potential issue | 🟡 Minor

Reintroduced Biome useIterableCallbackReturn lint violations.

The forEach callbacks still use implicit returns. This was previously flagged and appears reintroduced here.

🛠️ Proposed fix

-            root.querySelectorAll("audio").forEach((player) => callback(player));
+            root.querySelectorAll("audio").forEach((player) => {
+                callback(player);
+            });

             root.querySelectorAll("input.volume-slider[type='range'], input#volume[type='range']").forEach(
-                (slider) => callback(slider)
+                (slider) => {
+                    callback(slider);
+                }
             );

#!/bin/bash
# Verify implicit-return callbacks in forEach are gone.
# Expected: no matches after applying the fix.
rg -nP --type=js '\.forEach\(\(\w+\)\s*=>\s*callback\(\w+\)\)' acestep/ui/gradio/interfaces/audio_player_preferences.js

Also applies to: 86-88

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@acestep/ui/gradio/interfaces/audio_player_preferences.js` around lines 76 -
77, Replace implicit-return arrow callbacks passed to forEach with block-bodied
arrow functions; e.g., change the call in
root.querySelectorAll("audio").forEach((player) => callback(player)) to use a
block body like root.querySelectorAll("audio").forEach((player) => {
callback(player); }); Do the same for the other occurrences referenced (around
the similar forEach at lines ~86-88) so no forEach callbacks use implicit
returns.

acestep/ui/gradio/events/results/generation_info.py (1)

40-40: Use iterable unpacking instead of tuple concatenation (Ruff RUF005).

🛠️ Proposed fix

-    return tuple(gr.update(playback_position=0) for _ in range(8)) + (None,)
+    return (*[gr.update(playback_position=0) for _ in range(8)], None)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@acestep/ui/gradio/events/results/generation_info.py` at line 40, Replace the
tuple concatenation with iterable unpacking to satisfy Ruff RUF005: in the
return statement that currently builds a tuple from
(gr.update(playback_position=0) for _ in range(8)) + (None,), change it to use
starred unpacking so the generated items and the trailing None are combined via
unpacking (i.e., use a single return that unpacks the generator expression and
appends None) — update the statement in generation_info.py where
gr.update(playback_position=0) is produced.

acestep/ui/gradio/interfaces/audio_player_preferences.js (1)

12-13: Consider pruning disconnected roots from knownRoots.

knownRoots only grows and keeps strong references to detached roots, which can accumulate across remount-heavy sessions and increase scan cost over time. Pruning stale roots before iteration would make scans more stable long-term.

Also applies to: 205-215, 228-249

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@acestep/ui/gradio/interfaces/audio_player_preferences.js` around lines 12 -
13, knownRoots currently accumulates detached roots; before any iteration over
knownRoots (e.g., the loops around the areas you flagged) prune it by filtering
out stale roots (check root.isConnected === false or !document.contains(root) /
!root.ownerDocument?.contains(root)) and for each removed root also delete its
entry from readyForPersistence (readyForPersistence.delete(root)); update any
in-place iterations to use the pruned array (or mutate knownRoots in-place) so
scans only touch connected roots and memory/scan cost won't grow unbounded.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

acestep/ui/gradio/events/results/generation_info.py (1)

25-40: ⚠️ Potential issue | 🟡 Minor

Re-add an explicit return type on clear_audio_outputs_for_new_generation.

This modified function lost its return annotation; please restore it to keep typing/tooling consistent and avoid regressions from the prior fix.

🔧 Proposed fix

-def clear_audio_outputs_for_new_generation():
+def clear_audio_outputs_for_new_generation() -> tuple[Any, ...]:

#!/bin/bash
python - <<'PY'
import ast
from pathlib import Path

p = Path("acestep/ui/gradio/events/results/generation_info.py")
tree = ast.parse(p.read_text(encoding="utf-8"))

fn = next(
    n for n in tree.body
    if isinstance(n, ast.FunctionDef) and n.name == "clear_audio_outputs_for_new_generation"
)
print("return_annotation_present:", fn.returns is not None)
PY

As per coding guidelines: "Add type hints for new/modified functions when practical in Python".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@acestep/ui/gradio/events/results/generation_info.py` around lines 25 - 40,
The function clear_audio_outputs_for_new_generation lost its return type
annotation; restore a precise typing return (e.g. add "->
Tuple[Optional[object], ...]" or "-> Tuple[Any, ...]" and import the necessary
names from typing) so tooling knows the function returns a 9-tuple of
updates/None; update the top of the file to import Tuple, Optional, Any as
needed and add the annotation to the clear_audio_outputs_for_new_generation
definition.

acestep/ui/gradio/events/results/generation_info_test.py (1)

13-31: ⚠️ Potential issue | 🟡 Minor

Scope sys.modules stubs to avoid cross-test state leakage.

_load_module() mutates sys.modules globally; this can bleed into other tests. Wrap stub injection in patch.dict and keep it local to module execution.

🔧 Proposed fix

-def _load_module():
+def _load_module() -> types.ModuleType:
     """Load target module directly by file path for isolated testing."""
-    # Stub i18n package path so importing generation_info.py does not require
-    # importing the full Gradio UI package in headless test environments.
-    acestep_pkg = sys.modules.setdefault("acestep", types.ModuleType("acestep"))
-    ui_pkg = sys.modules.setdefault("acestep.ui", types.ModuleType("acestep.ui"))
-    gradio_pkg = sys.modules.setdefault("acestep.ui.gradio", types.ModuleType("acestep.ui.gradio"))
-    i18n_mod = types.ModuleType("acestep.ui.gradio.i18n")
-    i18n_mod.t = lambda key, **_kwargs: key
-    sys.modules["acestep.ui.gradio.i18n"] = i18n_mod
-    acestep_pkg.ui = ui_pkg
-    ui_pkg.gradio = gradio_pkg
-    gradio_pkg.i18n = i18n_mod
+    stub_modules = {
+        "acestep": types.ModuleType("acestep"),
+        "acestep.ui": types.ModuleType("acestep.ui"),
+        "acestep.ui.gradio": types.ModuleType("acestep.ui.gradio"),
+        "acestep.ui.gradio.i18n": types.ModuleType("acestep.ui.gradio.i18n"),
+    }
+    stub_modules["acestep.ui.gradio.i18n"].t = lambda key, **_kwargs: key
+    stub_modules["acestep"].ui = stub_modules["acestep.ui"]
+    stub_modules["acestep.ui"].gradio = stub_modules["acestep.ui.gradio"]
+    stub_modules["acestep.ui.gradio"].i18n = stub_modules["acestep.ui.gradio.i18n"]
 
     module_path = Path(__file__).with_name("generation_info.py")
     spec = importlib.util.spec_from_file_location("generation_info", module_path)
     module = importlib.util.module_from_spec(spec)
-    spec.loader.exec_module(module)  # type: ignore[union-attr]
+    with patch.dict(sys.modules, stub_modules):
+        spec.loader.exec_module(module)  # type: ignore[union-attr]
     return module

#!/bin/bash
python - <<'PY'
from pathlib import Path
import re

src = Path("acestep/ui/gradio/events/results/generation_info_test.py").read_text(encoding="utf-8")
m = re.search(r"def _load_module\(\):(.+?)return module", src, flags=re.S)
body = m.group(1) if m else ""
print("uses_sys_modules_setdefault:", "sys.modules.setdefault" in body)
print("uses_patch_dict_scope:", "with patch.dict(sys.modules" in body or 'with patch.dict("sys.modules"' in body)
PY

As per coding guidelines: "Avoid hidden state and unintended side effects in Python code".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@acestep/ui/gradio/events/results/generation_info_test.py` around lines 13 -
31, The test helper _load_module currently mutates sys.modules via
sys.modules.setdefault and assigns attributes on the stub packages (acestep,
acestep.ui, acestep.ui.gradio, acestep.ui.gradio.i18n), which can leak state
across tests; change _load_module to create the same stub entries but inside a
with patch.dict(sys.modules, {...}, clear=False): context so the temporary
modules (keys like "acestep", "acestep.ui", "acestep.ui.gradio",
"acestep.ui.gradio.i18n") are injected only while you call
spec.loader.exec_module(module) and are automatically reverted afterwards—keep
the existing i18n_mod.t lambda and the assignments to module attributes
(acestep_pkg.ui, ui_pkg.gradio, gradio_pkg.i18n) but move them into the
patch.dict block and remove direct sys.modules.setdefault calls outside the
context to avoid cross-test leakage in _load_module.

acestep/ui/gradio/interfaces/audio_player_preferences.js (1)

76-77: ⚠️ Potential issue | 🟡 Minor

Reapply block-bodied forEach callbacks to satisfy Biome.

These callbacks implicitly return a value and currently trigger lint/suspicious/useIterableCallbackReturn.

🛠️ Proposed fix

-            root.querySelectorAll("audio").forEach((player) => callback(player));
+            root.querySelectorAll("audio").forEach((player) => {
+                callback(player);
+            });
...
-            root.querySelectorAll("input.volume-slider[type='range'], input#volume[type='range']").forEach(
-                (slider) => callback(slider)
-            );
+            root.querySelectorAll("input.volume-slider[type='range'], input#volume[type='range']").forEach(
+                (slider) => {
+                    callback(slider);
+                }
+            );

#!/bin/bash
# Expect: no matches after applying the fix.
rg -nP 'forEach\(\((player|slider)\)\s*=>\s*callback\(' acestep/ui/gradio/interfaces/audio_player_preferences.js

Also applies to: 86-88

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@acestep/ui/gradio/interfaces/audio_player_preferences.js` around lines 76 -
77, The Biome lint flags the concise arrow callbacks used with forEach because
they implicitly return a value; update the callbacks to use a block body so they
return undefined explicitly. Replace the single-expression arrow callbacks like
root.querySelectorAll("audio").forEach((player) => callback(player)); with
block-bodied equivalents that call callback inside a brace-delimited function
(e.g., (player) => { callback(player); }) and do the same for the other affected
forEach usages (the similar slider/element loops around lines 86-88) ensuring
you update the callbacks that invoke callback(...) to use { callback(...); }
form.

acestep/ui/gradio/interfaces/audio_player_preferences.js (1)

12-13: Prune detached roots to avoid long-session root retention.

knownRoots holds strong references indefinitely; after repeated remounts, stale roots can accumulate and increase scan/sync cost.

♻️ Proposed refactor

 const knownRoots = [];
+const isRootConnected = (root) => {
+    if (!root) {
+        return false;
+    }
+    if (root === document) {
+        return true;
+    }
+    if (root.host && typeof root.host.isConnected === "boolean") {
+        return root.host.isConnected;
+    }
+    return root.isConnected === true;
+};
+
+const pruneKnownRoots = () => {
+    for (let i = knownRoots.length - 1; i >= 0; i -= 1) {
+        const root = knownRoots[i];
+        if (!isRootConnected(root)) {
+            knownRoots.splice(i, 1);
+        }
+    }
+};
...
 const stopAndRewindAllPlayers = () => {
     discoverRoots();
+    pruneKnownRoots();
     forEachAudioPlayer((player) => {
         if (!player) {
             return;
         }
...
 const scanPlayers = () => {
     discoverRoots();
+    pruneKnownRoots();
     forEachAudioPlayer(registerPlayer);
     forEachVolumeSlider(registerVolumeSlider);
     syncAllVolumeControlsToPreferred();
 };

Also applies to: 200-206

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@acestep/ui/gradio/interfaces/audio_player_preferences.js` around lines 12 -
13, knownRoots currently holds strong references and should be changed to avoid
retaining detached roots; replace the array-based knownRoots with a WeakSet
(e.g., knownRoots = new WeakSet()) and update all places that add/remove/iterate
roots to use weak membership semantics (use knownRoots.add(root) /
knownRoots.delete(root)) and avoid relying on iteration over the WeakSet—when
you need to enumerate live roots, derive the list by filtering the existing
iterable you already have (or filter any array by checking root.isConnected) or
use readyForPersistence (WeakMap) keys to determine liveness; also apply the
same replacement/pruning logic for the other occurrence around the 200–206
region so stale roots are not retained.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro